chore(avm)!: Bytecode Retrieval pre-audit docs #20718
chore(avm)!: Bytecode Retrieval pre-audit docs #20718MirandaWood merged 11 commits intomerge-train/avmfrom
Conversation
barretenberg/cpp/src/barretenberg/vm2/simulation/events/bytecode_events.hpp
Outdated
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/tracegen/bytecode_trace.cpp
Outdated
Show resolved
Hide resolved
c3592ae to
fd65309
Compare
fd65309 to
82a758d
Compare
barretenberg/cpp/src/barretenberg/vm2/simulation/events/bytecode_events.hpp
Outdated
Show resolved
Hide resolved
| // We convert the bytecode to a shared_ptr because it will be shared by some events. | ||
| auto shared_bytecode = std::make_shared<std::vector<uint8_t>>(std::move(klass.packed_bytecode)); | ||
| // Emits BytecodeDecompositionEvent. | ||
| decomposition_events.emit({ .bytecode_id = bytecode_id, .bytecode = shared_bytecode }); |
There was a problem hiding this comment.
The fact that we emit "decomposition events" suggest that the circuit does not map simulation in an equivalent way. bc_retrieval.pil does not have interaction with bc_decomposition.pil IIRC.
Did you reason about the completeness at "higher level" of the mechanism of "bc retrieval, fetching, decomposition, hashing"? It looks like it is not fully modular and "local review" might not be sufficient.
There was a problem hiding this comment.
So yes, in terms of the 'map' I added to the top:
* execution.pil --> bc_retrieval.pil --> contract_instance_retrieval.pil --> nullifier_check.pil
* --> address_derivation.pil
* --> update_check.pil
* --> retrieved_bytecodes_tree_check.pil
* --> class_id_derivation.pil
* --> instr_fetching.pil --> bc_decomposition.pil <-> bc_hashing.pil
* --> precomputed.pil
The lookup execution.pil --> bc_retrieval.pil occurs on the first row of each context (sel_first_row_in_context) regardless of any errors. The instruction fetching lookup occurs whenever we have sel_bytecode_retrieval_success which IIRC is a 'subset' of sel_first_row_in_context (i.e. it's only on at the first row of each context, but not vice versa).
I think that the decomposition event emission in simulation maps 1:1 to the circuit behaviour since we emit it when:
- we have already emitted a retrieval event (i.e. can't have a decomp event without a retrieval event)
- there is no error, otherwise we return early (
sel_bytecode_retrieval_success== 0 in this case) - we have already accessed this bytecode (
is_new_class == 0and a decomp event will have already been emitted in this case)
I do think at the instruction fetching level this is hard to reason about, since in the circuit we link instr_fetching.pil --> bc_decomposition.pil but in simulation we 'link' bytecode_manager (=> bc_retrieval, bc_decomposition, bc_hashing) to the decomp event. It's the higher level 'link' at simulation's execute which puts together instruction fetching and bytecode management. So it works in a very roundabout way.
Maybe for the future we should consider moving emitting the bytecode decomp event via the instruction fetching call, to make it easier to reason about?
barretenberg/cpp/src/barretenberg/vm2/tracegen/bytecode_trace.cpp
Outdated
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/tracegen/bytecode_trace.cpp
Outdated
Show resolved
Hide resolved
… pass preaudit WIP
ad46454 to
701e9ed
Compare
jeanmon
left a comment
There was a problem hiding this comment.
Great! I have only minor suggestions.
Bytecode retrieval pre-audit PR including:
This trace does a lot of delegation, so the pre audit has been completed assuming the target circuits constrain as expected! The actual columns used in these lookups have been checked though.
Closes AVM-54
TODOs/Notes: See comments for those to discuss (otherwise, I need to add
preconditions, doc on why we zero some properties on error, and tracegen testsUpdate: complete!).